Feat/#513, #523 검색완료, 가수 상세 페이지 구현 및 메인페이지 케러셀 정책 변경 반영 #525

Creative-Lee · 2023-10-19T03:46:03Z

📝작업 내용

검색완료, 가수 상세 페이지를 구현 했습니다.
메인페이지 케러셀 정책 변경사항을 반영했습니다.

💬리뷰 참고사항

작업 한 2개의 페이지는 view가 같아 컴포넌트를 재사용하는 방향으로 구현해 보았습니다.
- 컴포넌트가 잘 분리 되었는지 확인해주시면 감사하겠습니다.
메인 페이지 케러셀에 바뀐 정책을 반영하는 커밋도 함께 들어가 있습니다.
- remote 함수 추가
- msw 픽스쳐, 핸들러 추가
- 케러셀 컴포넌트의 투표중인 노래 관련 네이밍을 최근 등록된 노래로 변경
- 케러셀 클릭 시 해당 노래 듣기 페이지로 이동(장르 : ALL)
- 더이상 로그인 검증이 필요하지 않으므로(듣기 페이지 이동은 누구나 가능) 관련 분기 삭제
스토리북 각 스토리 title에 prefix를 달아서 정리해 놓았습니다.

#️⃣연관된 이슈

연관된 이슈 번호를 모두 작성

(close) #이슈번호

리스트 컴포넌트 분리

스토리북에서 li요소 단일 랜더 시 ::marker 표시 이슈로 li list styled none 추가

github-actions · 2023-10-19T09:00:26Z

Unit Test Results

  90 files   90 suites 12s ⏱️
333 tests 333 ✔️ 0 💤 0 ❌
336 runs 336 ✔️ 0 💤 0 ❌

Results for commit 5dbd153.

♻️ This comment has been updated with latest results.

cruelladevil

여러 페이지 작업하느라 고생 많으셨습니다 👍👍
대부분 개인적 의견이라 다음에 반영해주셔도 됩니다.
린트 에러와 띄어쓰기는 꼭 고치고 머지해주세용

cruelladevil · 2023-10-19T07:18:01Z

frontend/src/shared/types/song.ts

💬 여기 폴더만 파일들이 type이 안붙네요.
명확하게 정하고 가면 좋을 것 같아요.

types 폴더 안에 있으니 안붙혀도 된다.

그래도 붙히자.

cruelladevil · 2023-10-19T07:24:26Z

frontend/src/shared/components/ToggleSwitch/ToggleSwitch.stories.tsx

@@ -3,7 +3,7 @@ import type { Meta, StoryObj } from '@storybook/react';

 const meta = {
  component: ToggleSwitch,
-  title: 'ToggleSwitch',
+  title: 'shared/ToggleSwitch',


👍 경로 설정해서 스토리북에서 묶어주셨군요

cruelladevil · 2023-10-19T07:32:19Z

frontend/src/shared/types/song.ts

+export interface RecentSong {
+  id: number;
+  title: string;
+  singer: string;
+  videoLength: number;
+  albumCoverUrl: string;
+}


💬 저희 겹치는 타입들이 너무 많은 것 같아요
의존성을 갖게 할 타입들을 의존성을 갖게하고, 반복하지 않아도 될 부분을 반복하지 않는 리팩토링이 필요할 것 같네요.

// 의존성이 필요한 예제 interface RecentSong { id: SongDetail['id'] title: SongDetail['title'] singer: SongDetail['singer'] videoLength: SongDetail['videoLength'] albumCoverUrl: SongDetail['albumCoverUrl'] } // 반복도 줄임 type RecentSong = Pick<SongDetail, 'id' | 'title' | 'singer'>;

물론 꼭 의존성을 가져야 하는 것이 답이 아닙니다. 분리되어 관리되어야 한다면 분리하는 것도 맞는 것이죠
정책 변경과 기능 변경에 따라 api 응답들도 달라지면서 사용하지 않는 부분들도 있는데요.
그 부분을 리팩토링 하면서 타입도 함께 정리하면 좋을 것 같아요.

맞습니다~ 타입이 겹치는 부분이 꽤나 많더라고요.
개인적으로는 서버에서 취급하는 앤티티 대로 type을 만드는게 좋다고 생각합니당
나중에 리팩토링 해보죠!

cruelladevil · 2023-10-19T07:45:54Z

frontend/src/pages/SingerDetailPage.tsx

 import useValidParams from '@/shared/hooks/useValidParams';

 const SingerDetailPage = () => {
  const { singerId } = useValidParams();

-  return <div>{singerId}</div>;
+  const { data: singerDetail } = useFetch(() => getSingerDetail(Number(singerId)));
+  if (!singerDetail) return;


💬 저희 return null로 안하고 return만 한 곳이 있긴 하네요. (둘다 컴포넌트를 안그리는 것은 똑같습니다)
해당 부분은 통일해봐도 괜찮을 것 같네요.

https://github.com/woowacourse-teams/2023-shook/blob/main/frontend/src/features/songs/components/KillingPartInfo.tsx#L18

Suggested change

if (!singerDetail) return;

if (!singerDetail) return null;

null을 명시하면 더 직관적이고 좋을것 같네요! 변경할게요!

cruelladevil · 2023-10-19T07:57:23Z

frontend/src/features/search/remotes/search.ts

💬 /search/remotes/search.ts와
/search/remotes/singer.ts는
search feature가 맞는 걸까요.. singer feature가 맞는 걸까요..

search라고 하기엔 api 엔드포인트는 singer이고,
singer라고 하기엔 search에 사용되는 비동기통신이고,

getSingerDetail은 singer에 더 맞아보이고, 쪼금 모호해 보이네요 😭

아주 좋은 의견 감사합니다.
저도 구현하면서 해당 부분에 의문이 있었는데요~ 코난이 코멘트로 리마인드해서 바론에게 요청했습니다 ㅎㅎ

엔드 포인트가 singer로 시작하는 검색 api는 다음과 같이 변경되었습니다!

개선 전 /singers?name={query}&search=singer /singers?name={query}&search=singer&search=song

개선 후 /search?keyword={query}&type=singer /search?keyword={query}&type=singer&type=song

개선후에도, 의미적으로 어색한 부분이 존재하는데요~

저는 type이 의미하는 바가 무엇이지?라는 생각이 들더라고요!
검색할수 있는 범위를 나타내는 듯 하게 보여서 remote 함수 네이밍과 혼동이 생길수도 있어요

이 부분은 우선 1차 개선까지만 하고 추후에 고도화해서 반영하기로 했습니다!

또 추가로... ㅋㅋㅋ

search/remotes/ 하위 getSingerSearch의 반환타입은 SingerDetail []이고
SingerDetail type은 singer/types/ 에 존재해요.

remote 함수 자체는 검색을 위한, 함수이기에 search/remotes/ 하위에 위치하고
반환타입은 가수의 상세정보이기에 singer/types/ 하위에 위치하도록 했습니다.

정책에 대해 어떻게 생각하느냐에 따라 다르게 생각할 수 있는 부분이라고 생각합니다

cruelladevil · 2023-10-19T08:58:14Z

frontend/src/pages/MainPage.tsx


  return (
    <Container>
      <SRHeading>shook 메인 페이지</SRHeading>
      <Title>현재 킬링파트 등록중인 노래</Title>
      <Spacing direction="vertical" size={16} />
      <CollectionCarousel>
-        {isEmptyVotingSongs ? (
-          <EmptyMessage>


사용하지 않아서 스타일드 컴포넌트 린트 에러 납니당

해결 완!

cruelladevil · 2023-10-19T08:58:36Z

frontend/src/pages/SingerDetailPage.tsx

+import styled from 'styled-components';
+import { getSingerDetail } from '@/features/search/remotes/singer';
+import SingerBanner from '@/features/singer/components/SingerBanner';
+import SingerSongItem from '@/features/singer/components/SingerSongItem';


사용하지 않아서 린트 에러 뜹니다 222

cruelladevil · 2023-10-19T09:00:10Z

frontend/src/pages/SearchResultPage.tsx

💬 SearchResultPage와 SingerDetailPage의 디자인을 구분하기가 너무 어려워요..😭

최대한 조금이라도 구별점을 주기 위해 타이틀을 다르게 개선했습니다.

추가적인 디자인 개선도 추후에 해봅시다~

cruelladevil · 2023-10-19T09:01:20Z

frontend/src/shared/styles/GlobalStyles.ts

@@ -61,7 +61,7 @@ const GlobalStyles = createGlobalStyle`
    border-collapse: collapse;
  }

-  ol, ul { 
+  ol, ul, li { 


👍 storybook에서 단일 아이템에 대한 list-style 제거 용도 확인하였습니다.

cruelladevil · 2023-10-19T09:08:33Z

frontend/src/shared/components/Spacing.tsx

+const spacingCss = (
+  spacing?: Partial<Spacing>,
+  originalDir?: Spacing['direction'],
+  originalSize?: Spacing['size']
+) => {
+  if (!spacing) return;
+  const { direction: newDirection, size: newSize } = spacing;
+
+  const realDirection = newDirection ?? originalDir;
+  const realSize = newSize ?? originalSize;
+
+  return css`
+    min-width: ${realDirection === 'horizontal' ? `${realSize}px` : '0'};
+    min-height: ${realDirection === 'vertical' ? `${realSize}px` : '0'};
+  `;
+};


💬 해당 함수를 이해하는게 힘들었어용..🥲
real은 뭐지.. new는 뭐지.. 하다가 아래와 같이 제안드립니다.

Suggested change

const spacingCss = (

spacing?: Partial<Spacing>,

originalDir?: Spacing['direction'],

originalSize?: Spacing['size']

) => {

if (!spacing) return;

const { direction: newDirection, size: newSize } = spacing;

const realDirection = newDirection ?? originalDir;

const realSize = newSize ?? originalSize;

return css`

min-width: ${realDirection === 'horizontal' ? `${realSize}px` : '0'};

min-height: ${realDirection === 'vertical' ? `${realSize}px` : '0'};

`;

};

const spacingCss = (

responsiveSpacing?: Partial<Spacing>,

originalDir?: Spacing['direction'],

originalSize?: Spacing['size']

) => {

if (!responsiveSpacing) return;

const direction = responsiveSpacing.direction ?? originalDir;

const size = responsiveSpacing.size ?? originalSize;

return css`

min-width: ${direction === 'horizontal' ? `${size}px` : '0'};

min-height: ${direction === 'vertical' ? `${size}px` : '0'};

`;

};

코드 설명을 달아놨어야 하는데, 깜빡했네요!!
저도 코난 의견에 동의합니다!
Flex 컴포넌트에 적용된 방식대로 인자명을 비슷하게 가져가려고 하다보니 읽기 어려워졌네요!

적극 반영하였습니다~

검색 api 엔드포인트 변경 singer -> search / 검색 의미 강조하고자 함 name -> keyword / 범용적인 검색어의 의미를 주고자 함 search -> type / 검색 타입의 의미를 주고자함

Creative-Lee added 30 commits October 17, 2023 22:23

feat: 가수 상세 페이지 구현

9c0abec

refactor: 핸들러 로직 수정

f7ebde1

design: 반응형 디자인 추가

f8eb5c5

chore: type 파일 suffix 추가

4ed3b2b

refactor: 가수 디테일과 검색결과 type 분리

9e40f33

design: 미디어 쿼리 호버 적용

111c2e5

chore: type 파일 이동

b44dbab

refactor: SingersSong type 추가

3873123

refactor: 컴포넌트 분리

954d871

test: 스토리 추가

65d0f1a

design: 분리된 컴포넌트 색상 추가

91a580a

refactor: Spacing 컴포넌트 반응형 디자인 props 추가

749c9db

feat: 노래 클릭 시 듣기페이지 이동 기능 구현

217b9f4

refactor: 컴포넌트 분리

3e72fab

리스트 컴포넌트 분리

refactor: 하위 컴포넌트가 title을 가지도록 변경

edc9df0

feat: 검색 결과 페이지 구현

ca30a5f

chore: 노래 검색결과 픽스쳐 데이터 수정

c200e1f

feat: 배너 클릭 시 가수페이지 이동 기능 구현

41d756c

chore: 사용하지 않는 import 삭제 및 픽스쳐 데이터 삭제

9db9be2

feat: 배너 호버 시 아이콘 디테일 추가

2433293

design: 아이콘 사이즈 축소

56e7ab5

chore: 사용하지 않는 훅 삭제

558733f

feat: 메인 케러셀 최근 추가된 노래 리스트로 대체

5306d46

feat: 메인 케러셀 최근 추가된 노래 리스트로 대체

d170a1a

feat: 최근 들은 노래 핸들러 및 fixture 구현

dd78f3c

design: 글로벌 스타일 수정

5afc6d2

스토리북에서 li요소 단일 랜더 시 ::marker 표시 이슈로 li list styled none 추가

test: singer관련 스토리 추가

6417256

design: title color 추가

19518c8

test: 스토리 title prefix 추가로 디렉터리 구분

16fd725

design: 기본 배경색 지정

0c108bd

Creative-Lee added [ 🌞 FE ] 프론트엔드 크루들의 빛나는 개발 이야기 하나둘셋 호! ✨ Feat 꼼꼼한 기능 구현 중요하죠 labels Oct 19, 2023

Creative-Lee added this to the [6차] 서비스 고도화, 정책 변경에 따른 기능 추가 및 개선 milestone Oct 19, 2023

Creative-Lee self-assigned this Oct 19, 2023

Creative-Lee requested review from ukkodeveloper and cruelladevil as code owners October 19, 2023 03:46

Merge branch 'main' into feat/#513

7da9bb3

cruelladevil approved these changes Oct 19, 2023

View reviewed changes

Creative-Lee and others added 10 commits October 19, 2023 18:11

fix: lint 에러 해결

1597a23

refactor: spacing 컴포넌트 변수명 변경으로 가독성 개선

537b832

refactor: null 명시

5527cf6

chore: singer 리모트 함수 파일 위치 변경

fae6725

refactor: api 명세 변경으로 의미 개선

6afd8b0

검색 api 엔드포인트 변경 singer -> search / 검색 의미 강조하고자 함 name -> keyword / 범용적인 검색어의 의미를 주고자 함 search -> type / 검색 타입의 의미를 주고자함

refactor: api 명세 변경 msw 반영 및 singer 핸들러 분리

db32c8a

refactor: 함수 불리언 화 방식 변경

29eae71

design: 페이지 별 title을 다르게 하여 디자인 구분 되로록 개선

5dbd153

refactor: params가 변경되면 데이터를 refetch하도록 변경

0b989a1

Merge branch 'main' into feat/#513

cd49437

Creative-Lee merged commit bab208b into main Oct 19, 2023
2 checks passed

Creative-Lee deleted the feat/#513 branch October 19, 2023 13:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/#513, #523 검색완료, 가수 상세 페이지 구현 및 메인페이지 케러셀 정책 변경 반영 #525

Feat/#513, #523 검색완료, 가수 상세 페이지 구현 및 메인페이지 케러셀 정책 변경 반영 #525

Creative-Lee commented Oct 19, 2023 •

edited

Loading

github-actions bot commented Oct 19, 2023 •

edited

Loading

cruelladevil left a comment

cruelladevil Oct 19, 2023

cruelladevil Oct 19, 2023

cruelladevil Oct 19, 2023

Creative-Lee Oct 19, 2023

cruelladevil Oct 19, 2023

Creative-Lee Oct 19, 2023

cruelladevil Oct 19, 2023

Creative-Lee Oct 19, 2023 •

edited

Loading

Creative-Lee Oct 19, 2023

Creative-Lee Oct 19, 2023

cruelladevil Oct 19, 2023

Creative-Lee Oct 19, 2023

cruelladevil Oct 19, 2023

cruelladevil Oct 19, 2023 •

edited

Loading

Creative-Lee Oct 19, 2023

Creative-Lee Oct 19, 2023

cruelladevil Oct 19, 2023

cruelladevil Oct 19, 2023

Creative-Lee Oct 19, 2023

Feat/#513, #523 검색완료, 가수 상세 페이지 구현 및 메인페이지 케러셀 정책 변경 반영 #525

Feat/#513, #523 검색완료, 가수 상세 페이지 구현 및 메인페이지 케러셀 정책 변경 반영 #525

Conversation

Creative-Lee commented Oct 19, 2023 • edited Loading

📝작업 내용

💬리뷰 참고사항

#️⃣연관된 이슈

github-actions bot commented Oct 19, 2023 • edited Loading

Unit Test Results

cruelladevil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Creative-Lee Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cruelladevil Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Creative-Lee commented Oct 19, 2023 •

edited

Loading

github-actions bot commented Oct 19, 2023 •

edited

Loading

Creative-Lee Oct 19, 2023 •

edited

Loading

cruelladevil Oct 19, 2023 •

edited

Loading